-
Notifications
You must be signed in to change notification settings - Fork 282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Part 1) stream-chat-android-client
unit tests - api
package
#5660
base: develop
Are you sure you want to change the base?
Conversation
Stream-chat-android-client
unit tests - api
packagestream-chat-android-client
unit tests - api
package
SDK Size Comparison 📏
|
@Test | ||
fun testApiKeyIsAddedAsHeader() { | ||
// given | ||
val apiKey = "apiKeyValue" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val apiKey = "apiKeyValue" | |
val apiKey = randomString() |
@Test | ||
fun testAnonymousUserHeaders() { | ||
// given | ||
val isAnonymous = { true } | ||
val headersUtil = mock<HeadersUtil>() | ||
whenever(headersUtil.buildSdkTrackingHeaders()).doReturn("sdkTrackingHeaders") | ||
whenever(headersUtil.buildUserAgent()).doReturn("userAgent") | ||
val interceptor = HeadersInterceptor(isAnonymous, headersUtil) | ||
// when | ||
val response = interceptor.intercept(FakeChain(FakeResponse(200))) | ||
// then | ||
response.request.header("User-Agent") `should be equal to` "userAgent" | ||
response.request.header("Content-Type") `should be equal to` "application/json" | ||
response.request.header("stream-auth-type") `should be equal to` "anonymous" | ||
response.request.header("X-Stream-Client") `should be equal to` "sdkTrackingHeaders" | ||
response.request.header("Cache-Control") `should be equal to` "no-cache" | ||
} | ||
|
||
@Test | ||
fun testAuthenticatedUserHeaders() { | ||
// given | ||
val isAnonymous = { false } | ||
val headersUtil = mock<HeadersUtil>() | ||
whenever(headersUtil.buildSdkTrackingHeaders()).doReturn("sdkTrackingHeaders") | ||
whenever(headersUtil.buildUserAgent()).doReturn("userAgent") | ||
val interceptor = HeadersInterceptor(isAnonymous, headersUtil) | ||
// when | ||
val response = interceptor.intercept(FakeChain(FakeResponse(200))) | ||
// then | ||
response.request.header("User-Agent") `should be equal to` "userAgent" | ||
response.request.header("Content-Type") `should be equal to` "application/json" | ||
response.request.header("stream-auth-type") `should be equal to` "jwt" | ||
response.request.header("X-Stream-Client") `should be equal to` "sdkTrackingHeaders" | ||
response.request.header("Cache-Control") `should be equal to` "no-cache" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of writting the same test multiple times only because a single parameter is different, we can use @ParameterizedTest
providing an input source with the different input/expected values.
On the other hand, consider to use randomString()
when the value "is not important" and our code accepts whatever value, it could raise up hidden bubs in our code (hardcoded values, default behavior given a default value, ...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to write a @ParameterizedTest
for just 2 tests because I thought it would reduce the readability a bit. But I can change it anyway, since I will change some of the values with randomString()
// then | ||
verify(analyser, times(1)).registerRequest( | ||
requestName = chain.request().url.toString(), | ||
data = mapOf("body" to "no_body"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we only testing when the response is empty?
Should we test other cases?
As commented on another comment, we can use @ParameterizedTest
on that case.
val request = Request.Builder() | ||
.url("https://hello.url") | ||
.post("body".toRequestBody()) | ||
.tag(ProgressCallback::class.java, progressCallback) | ||
.build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about creating a Mother
method to easily create Request
with custom value?
import org.mockito.kotlin.mock | ||
import org.mockito.kotlin.whenever | ||
|
||
internal class DistinctChatApiTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we are only testing the case where arguments are the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each method is actually covered with 3 tests:
- same arguments, at the same time
- same arguments, called sequentially
- different arguments, at the same time
val call1 = distinctChatApi.queryChannel("messaging", "1", QueryChannelRequest()) | ||
val call2 = distinctChatApi.queryChannel("messaging", "1", QueryChannelRequest()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of hardcoded values, we should use random ones.
On the other hand, QueryChannelRequest
constructor is generating an "equal" instance, but we should verify it by initializing it with custom data, not only the default values.
|
🎯 Goal
Increase the unit test coverage of the project.
Cover the
api
package from thestream-chat-android-client
package🛠 Implementation details
Add tests (where applicable) for the
io.getstream.chat.android.client.api
package